Skip to content

[AgentX] DeepSeek-v4-pro llm-d NVL72-B200 - #2719

Open
ilmarkov wants to merge 8 commits into
mainfrom
imarkov/dsv4-pro-agentic
Open

[AgentX] DeepSeek-v4-pro llm-d NVL72-B200#2719
ilmarkov wants to merge 8 commits into
mainfrom
imarkov/dsv4-pro-agentic

Conversation

@ilmarkov

@ilmarkov ilmarkov commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

DeepSeek-V4-Pro FP4 B200 llmd-vllm AgentX benchmarks (DSpark)

Adds llm-d (llmd-vllm) SLURM-based AgentX benchmark configs for DeepSeek-V4-Pro-0813 (DSpark checkpoint) on B200/GB200.

New configs

Four recipes under benchmarks/multi_node/llm-d-recipes/agentic/:

  • disagg-gb200-1p1d-dep8-dep8-dspark — P/D disagg, 1 prefill DEP8 + 1 decode DEP8 (2 nodes). Always uses Mooncake (P2PHANDSHAKE embedded RDMA prefix-cache). Prefill gets 1 speculative token; decode gets 5.
  • agg-gb200-tp8-dspark — Aggregated TP8, 1 node. DSpark speculative decoding (5 tokens). No Mooncake.
  • agg-gb200-dep8-dspark — Aggregated DEP8, 1 node. DSpark speculative decoding (5 tokens). No Mooncake.
  • agg-gb200-dep8-dspark-mooncake — Same as above with Mooncake cross-run prefix-cache sharing (swept at higher concurrencies).

Infrastructure changes

  • server.sh: adds Mooncake detection (parses mooncake.store_config from recipe YAML, writes /tmp/mooncake_config.json); wires MultiConnector (Nixl + MooncakeStoreConnector) for both disagg and aggregated+Mooncake modes
  • job.slurm: aggregated mode support (DECODE_NODES=0): no pd-sidecar, no KV transfer in base mode; rank 0 takes EPP/Envoy coordinator role
  • Image: quay.io/rh-ee-imarkov/llm-d-nokube-vllm:dspark-0814-nightly — built on vllm-openai:nightly-fix-8a102759ba2 with DSpark patches baked in (flashinfer_sparse.py, flashinfer_all_reduce.py, fused_allreduce_gemma_rms_norm.py) and Mooncake transfer engine (==0.3.12.post1). EPP/sidecar bumped to v0.10.0.
  • runners/launch_b200-nscale-compat.sh (née launch_b200-dgxc.sh): llmd-vllm dispatch wired; B200-only v0.10.0 EPP binary override via LLMD_BIN_DIR; aggregated vs. disagg script selection on DISAGG

Config fixes

  • Model name corrected to deepseek-ai/DeepSeek-V4-Pro-0813 in master config
  • Fixed stale 8k1k/ prefix on GB200 llm-d recipe paths

Note

Medium Risk
Touches multinode KV routing (Nixl/MultiConnector/Mooncake), coordinator logic, and cluster launch paths; misconfiguration could break SLURM jobs or prefix-cache handoff, but scope is benchmark infrastructure rather than production serving.

Overview
Adds B200 llm-d (llmd-vllm) AgentX coverage for DeepSeek-V4-Pro-0813 (DSpark) in nvidia-master.yaml: P/D disagg (dsv4-fp4-b200-llmd-vllm-agentx, 1P+1D DEP8, Mooncake, EPP v0.10.0) and aggregated (dsv4-fp4-b200-llmd-vllm-agentx-agg, TP8 / DEP8 / DEP8+Mooncake with DECODE_NODES=0).

llm-d stack gains aggregated mode (DECODE_NODES=0): server.sh sets IS_AGGREGATED, skips pd-sidecar and default KV transfer unless Mooncake is enabled; rank 0 runs EPP/Envoy/bench; job.slurm avoids invalid decode leader IPs; endpoints.yaml emits prefill-only. Mooncake is driven from recipe mooncake.store_config (JSON + optional mooncake-transfer-engine pip) with MultiConnector (Nixl + MooncakeStoreConnector) for disagg and aggregated+Mooncake.

New agentic recipe YAMLs, B200 wrapper scripts (dsv4_fp4_b200_llmd-vllm-{disagg,agg}.sh), binaries-b200-v0.10.0.env, and extract-binaries.sh override via BINARIES_ENV_FILE. launch_b200-nscale-compat.sh adds a self-contained llmd-vllm path (enroot/pyxis, DISAGG-based script pick, v0.10.0 LLMD_BIN_DIR for disagg) and pins -0813 to /scratch/models/DeepSeek-V4-Pro-0813.

Reviewed by Cursor Bugbot for commit e648883. Bugbot is set up for automated code reviews on this repo. Configure here.

…DEP8)

Ports the agentX (llm-manifesto/agentx-mvp) DeepSeek-V4-Pro 1P-EP8/1D-EP8
K8s deployment to InferenceX's SLURM-based llmd-vllm framework on B200
(8 GPUs/node, so each DEP8 role fits on one node vs GB200's 4 GPUs/node
needing two). New master-config key dsv4-fp4-b200-llmd-vllm-agentx and
recipe carry the agentX EPP scheduling config (disagg-profile-handler's
`deciders:` shape, prefix-cache-affinity-filter, token-load-scorer, etc.,
ported verbatim from a real agentx-mvp results snapshot) and per-role
vLLM tuning, dropping the Kubernetes/manifesto-specific Mooncake KV chain
in favor of server.sh's existing plain NixlConnector.

Wires a self-contained FRAMEWORK==llmd-vllm branch into
runners/launch_b200-dgxc.sh (ahead of the srtctl/srt-slurm path used by
the other B200 multinode frameworks, which llm-d bypasses entirely),
mirroring launch_gb200-nv.sh's pyxis/enroot + slurm_utils.sh helpers
pattern. Pins a B200-only EPP/pd-sidecar v0.10.0 binary override
(LLMD_BIN_DIR) for the newer EPP plugin shape, isolated from GB200's
shared v0.9.0 binaries/path.

Co-Authored-By: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs


感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

ilmarkov and others added 4 commits August 26, 2026 16:34
- dsv4-fp4-b200-llmd-vllm-agentx: drop c256 from the 1P-DEP8/1D-DEP8
  conc-list per request.
- extract-binaries.sh: support BINARIES_ENV_FILE to source an alternate
  pins file, so a cluster-scoped router-version bump doesn't require
  editing the shared binaries.env (whose EPP_FROM_IMAGE/
  ROUTING_SIDECAR_IMAGE are plain assignments and can't be overridden
  via pre-exported env vars).
- Add binaries-b200-v0.10.0.env: B200-only v0.10.0 EPP/pd-sidecar pins,
  linux/amd64 platform (B200 is x86_64, unlike GB200's arm64 default),
  dedicated LLMD_BIN_DIR.
- launch_b200-dgxc.sh: fix the extraction command documented in the
  LLMD_BIN_DIR comment, which previously relied on the non-overridable
  env vars above and would have silently extracted v0.9.0 binaries.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sibling of the P/D disagg benchmark: one engine per topology does both
prefill and decode (1 node / 8 GPUs each), no P/D KV handoff.

- server.sh/job.slurm: add an aggregated mode (DECODE_NODES=0) - no
  decode role, no pd-sidecar, no --kv_transfer_config (there is nothing
  to hand KV off to), and the sole engine's rank 0 takes over the
  EPP/Envoy/bench coordinator duties normally owned by the decode
  leader. Also fixes the underlying DECODE_NODES=0 bugs: job.slurm's
  out-of-bounds IPS[] read for DECODE_LEADER_IP, and server.sh emitting
  a bogus empty-address decode-0 endpoint in endpoints.yaml.
- New recipes dsv4-fp4-b200-agentx-agg-{tp8,dep8}.yaml: per-role vLLM
  tuning ported from agentX's ix-agg-base/agg-tp8 and
  ix-agg-dep-base/agg-dep8, plus a minimal single-schedulingProfile EPP
  config (no disagg-profile-handler needed - llm-d auto-selects
  single-profile-handler for one profile). Needs no EPP router bump
  since every plugin used is already in the default v0.9.0 binaries.
- New dsv4_fp4_b200_llmd-vllm-agg.sh wrapper + launch_b200-dgxc.sh now
  branches its llmd-vllm wrapper-script choice on $DISAGG instead of
  hardcoding the disagg wrapper.
- New dsv4-fp4-b200-llmd-vllm-agentx-agg master-config key (disagg:
  false, prefill num-worker=1 / decode num-worker=0 per the
  process_result.py / dynamo-vllm aggregated convention). conc-lists
  mirror the agentX dspark historical runs (results_ilmarkov-2-ix-agg-
  tp8-dspark-v3-long, results_ilmarkov-ix-agg-dep8-dspark-1308-long).
- README.md + perf-changelog.yaml updated.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ilmarkov
ilmarkov marked this pull request as ready for review August 31, 2026 12:01
@ilmarkov
ilmarkov requested a review from a team August 31, 2026 12:01
@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ilmarkov's task in 13m 58s —— View job


Review of PR #2719 — DeepSeek-V4-Pro llm-d B200 (AgentX)

  • Gather context and read the full diff
  • Review benchmark scripts and llm-d recipes
  • Validate configs/nvidia-master.yaml changes (image accessibility, model prefixes)
  • Validate perf-changelog.yaml entry (position, pairing with config change)
  • Review launcher script changes (runners/launch_b200-nscale-compat.sh)
  • Post inline comments and final summary

Summary

Found 3 blocking and 2 warning issues (posted as inline comments):

  1. 🔴 dsv4-fp4-b200-llmd-vllm-agentx-agg uses disagg: false with prefill:/decode: search-space blocks — rejected by master-config validation (validation.py:860, requires worker: + num-nodes:) and would KeyError('worker') in multinode_worker_pair, breaking sweep generation for the entire master config.
  2. 🔴 The three new perf-changelog.yaml entries are inserted mid-file instead of appended to the end, two still have pr-link: .../PLACEHOLDER, and the descriptions don't match the diff (Mooncake [52,72] is on the agg key, not disagg; wrong launcher and recipe filenames).
  3. 🔴 The launcher's dsv4/fp4 MODEL_PATH candidate order prefers /scratch/models/DeepSeek-V4-Pro (staged for all other dsv4-fp4-b200 configs), so the new DeepSeek-V4-Pro-0813 configs would benchmark the old checkpoint while publishing results under the -0813 name.
  4. 🟡 llmd_enroot_uri_for_image produces a malformed enroot URI for repo:tag@sha256: images (tag not stripped) — latent, but it's the exact pin format the sibling GB200 image uses.
  5. 🟡 The v0.10.0 LLMD_BIN_DIR binary override applies to aggregated runs too, contradicting their router: 0.9.0 metadata once the shared bin dir is populated.

Doc-consistency nits (no inline comments): many comments reference runners/launch_b200-dgxc-slurm.sh (a pre-existing broken symlink; the dispatch actually lives in launch_b200-nscale-compat.sh) and stale recipe filenames (dsv4-fp4-b200-agentx-*.yaml in the README/recipes vs the actual agentic/*-gb200-*-dspark-agentic.yaml names, which themselves say gb200 despite being B200 recipes); the disagg recipe header still claims Mooncake was "NOT ported" while the recipe now carries a mooncake: block. Also, the new runner: b200-multinode label isn't registered in configs/runners.yaml, so --runner-node-filter invocations will silently skip these configs.

Positive notes: server.sh's aggregated-mode gating (IS_AGGREGATED, endpoints emission, coordinator takeover) is carefully done, the enroot-import-from-public-registry pattern is present, EP is recipe-conditional rather than hardcoded, and the DSV4 bench path includes --use-chat-template.


Comment thread runners/launch_b200-nscale-compat.sh
Comment thread configs/nvidia-master.yaml Outdated
framework: llmd-vllm
router: { name: llm-d-router, version: "0.9.0" }
multinode: true
disagg: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 BLOCKING: disagg: false on a multinode entry whose search-space uses prefill:/decode: fails master-config validation and matrix generation.

Why it matters: _validate_multinode_entry_scope (utils/matrix_logic/validation.py:860-869) rejects this shape outright: disagg=false requires one worker: (and num-nodes: in every search-space entry) and rejects prefill/decode. Even if validation were bypassed, multinode_worker_pair() (utils/matrix_logic/generate_sweep_configs.py:258) does benchmark[Fields.WORKER.value] for non-disagg entries → KeyError: 'worker'. Since generation/validation runs over the whole master file, this breaks sweep-config generation for every config, not just this key.

Fix: Restructure each of the three aggregated search-space arms to the aggregated multinode shape used elsewhere (e.g. kimik3-fp4-h200-vllm-agentic): a single worker: block plus num-nodes: 1, moving PREFILL_NODES=1/DECODE_NODES=0/GPUS_PER_NODE=8/CONFIG_FILE=... into the worker's additional-settings (the generator maps worker: → prefill with num-worker, and synthesizes decode: num-worker: 0, so GPU accounting still counts the 8 GPUs once). Fix this →

Comment thread perf-changelog.yaml Outdated
Comment on lines +6391 to +6397
- config-keys:
- dsv4-fp4-b200-llmd-vllm-agentx
description:
- "Add DeepSeek-V4-Pro FP4 B200 llmd-vllm P/D disagg benchmark (1P DEP8 + 1D DEP8, 2 nodes / 16 GPUs), porting the agentX (llm-manifesto/agentx-mvp) EPP scheduling config and per-role vLLM tuning"
- "Wire a FRAMEWORK==llmd-vllm dispatch into runners/launch_b200-dgxc-slurm.sh (self-contained, bypasses the srtctl/srt-slurm path used by the other B200 multinode frameworks)"
- "Pin a B200-only EPP/pd-sidecar v0.10.0 binary override (LLMD_BIN_DIR) for the disagg-profile-handler deciders: EPP shape; GB200's shared v0.9.0 binaries/path are untouched"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 BLOCKING: New perf-changelog.yaml entries must be appended to the END of the file, and two of the three new entries have pr-link: .../pull/PLACEHOLDER.

Why it matters: The file is read chronologically (oldest at top, newest at bottom). These three entries are inserted mid-file (after the #2650 entry, before ~240 lines of existing entries ending at #2787), which breaks the ordering. The PLACEHOLDER links (lines 6397 and 6406) also leave the changelog pointing nowhere.

Additionally, the third entry's description doesn't match the diff: it attributes the Mooncake [52, 72] arm to the disagg key (dsv4-fp4-b200-llmd-vllm-agentx), but in configs/nvidia-master.yaml the [52, 72] Mooncake arm is on the agg key while the disagg arm runs [64, 128, 192]; it also references a recipe file dsv4-fp4-b200-agentx-1p1d-dep8-dep8-mooncake.yaml that doesn't exist in this PR (actual: agentic/agg-gb200-dep8-dspark-mooncake-agentic.yaml / agentic/disagg-gb200-1p1d-dep8-dep8-dspark-agentic.yaml), and the first two entries name runners/launch_b200-dgxc-slurm.sh while the dispatch was actually added to runners/launch_b200-nscale-compat.sh.

Fix: Move all three entries to the bottom of the file, replace both PLACEHOLDER links with https://github.com/SemiAnalysisAI/InferenceX/pull/2719, and correct the descriptions (config keys, conc lists, file names). Fix this →

# the full node-math writeup.
dsv4-fp4-b200-llmd-vllm-agentx:
image: quay.io/rh-ee-imarkov/llm-d-nokube-vllm:dspark-0814-nightly
model: deepseek-ai/DeepSeek-V4-Pro-0813

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 BLOCKING: This is the first config declaring deepseek-ai/DeepSeek-V4-Pro-0813, but the launcher will almost certainly load the base DeepSeek-V4-Pro weights instead.

Why it matters: runners/launch_b200-nscale-compat.sh:23-35 resolves the dsv4/fp4 MODEL_PATH by taking the first existing directory among /scratch/models/DeepSeek-V4-Pro, .../DeepSeek-V4-Pro-NVFP4, .../DeepSeek-V4-Pro-0813. All other dsv4-fp4-b200-* configs (e.g. dsv4-fp4-b200-vllm, -sglang, -trt) run deepseek-ai/DeepSeek-V4-Pro on this same cluster, so /scratch/models/DeepSeek-V4-Pro is staged and wins the loop. The new benchmarks would then serve the old checkpoint while publishing results under DeepSeek-V4-Pro-0813 (MODEL_NAME="$MODEL"). This is the exact failure mode the qwen3.5 *NVFP4-V2 branch comment in the same launcher warns about (lines 43-52).

Fix: Add a checkpoint-specific branch in launch_b200-nscale-compat.sh before the generic dsv4/fp4 one, mirroring the qwen3.5 precedent:

elif [[ $MODEL_PREFIX == "dsv4" && $PRECISION == "fp4" && $MODEL == *-0813 ]]; then
    export MODEL_PATH="/scratch/models/DeepSeek-V4-Pro-0813"
    export SRT_SLURM_MODEL_PREFIX="deepseek-v4-pro"

(or reorder the candidate loop keyed on $MODEL). Fix this →

Comment on lines +153 to +156
if [[ "$registry" == "registry-1.docker.io" && "$repository" != */* ]]; then
repository="library/$repository"
fi
printf 'docker://%s#%s:%s\n' "$registry" "$repository" "$digest"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 WARNING: The digest branch keeps the tag inside repository, producing a malformed enroot URI for repo:tag@sha256:... images.

Why it matters: For an image like the sibling GB200 llm-d pin quay.io/rh-ee-imarkov/llm-d-nokube-vllm:vllm0.26@sha256:a909..., image_without_digest still contains :vllm0.26, so this prints docker://quay.io#rh-ee-imarkov/llm-d-nokube-vllm:vllm0.26:sha256:a909... — exactly the tag@digest ambiguity this function exists to avoid (per its own header comment). It's latent today only because the new B200 image is tag-only; the moment this image gets digest-pinned like its GB200 counterpart, the import breaks.

Fix:

Suggested change
if [[ "$registry" == "registry-1.docker.io" && "$repository" != */* ]]; then
repository="library/$repository"
fi
printf 'docker://%s#%s:%s\n' "$registry" "$repository" "$digest"
if [[ "$registry" == "registry-1.docker.io" && "$repository" != */* ]]; then
repository="library/$repository"
fi
# tag@digest form: drop the tag; enroot wants registry#repo:digest.
repository="${repository%%:*}"
printf 'docker://%s#%s:%s\n' "$registry" "$repository" "$digest"

Comment thread runners/launch_b200-nscale-compat.sh Outdated
# benchmarks/llm-d/extract-binaries.sh
# before this recipe can actually run; job.slurm's pyxis mount loop is
# a no-op (falls back to the image's baked-in v0.9.0) until then.
export LLMD_BIN_DIR="${LLMD_BIN_DIR:-/home/sa-shared/llm-d-bins-v0.10.0}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 WARNING: The v0.10.0 LLMD_BIN_DIR override is applied to every B200 llmd-vllm run, including the aggregated configs that declare router: { version: "0.9.0" }.

Why it matters: This export happens before the DISAGG branch, and job.slurm's mount loop mounts whatever executables it finds there over the image's baked-in binaries. Once /home/sa-shared/llm-d-bins-v0.10.0 is populated (a prerequisite for the disagg recipe to work at all), the dsv4-fp4-b200-llmd-vllm-agentx-agg runs will silently execute the v0.10.0 EPP/pd-sidecar while their master-config metadata, the recipe comments, and the perf-changelog all state they use the default v0.9.0 binaries — the published router version won't match what actually ran.

Fix: Either gate the override on the disagg path (move the export LLMD_BIN_DIR=... inside the DISAGG == true branch, added after the wrapper-selection if), or update the agg key's router: version/comments to 0.10.0 so the metadata matches reality.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond the inline findings, I checked the ruled-out changelog entry more closely: the Mooncake entry's config-keys (dsv4-fp4-b200-llmd-vllm-agentx) does match its "disagg DEP8 arm" description, since that key names the disagg config (not the -agg aggregated one), so there's no additional config-keys mismatch there.

Extended reasoning...

This run's bug hunt already surfaced five confirmed, non-trivial issues (env-var export bug breaking SLURM submission, append-only violation and invalid PLACEHOLDER pr-links in the byte-sensitive perf-changelog.yaml, a misnamed gb200 prefix on B200-only recipe files, and a malformed enroot URI for tag@ digest images), which on their own are sufficient to require human attention and block approval. I independently re-verified the one ruled-out candidate (the Mooncake entry's config-keys pointing at the disagg key dsv4-fp4-b200-llmd-vllm-agentx versus its "disagg DEP8 arm" description) by reading the surrounding perf-changelog.yaml entries directly, and confirmed the key correctly refers to the disagg config (the aggregated variant is the separate -agg key), so there is no mismatch there. Given the confirmed findings already speak for themselves as inline comments, this defer note is limited to recording that one additional, distinct concern was checked and ruled out rather than restating the existing findings.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment on lines +105 to +118
if [[ "$FRAMEWORK" == "llmd-vllm" ]]; then
# Self-contained: bypasses the srtctl/srt-slurm path entirely (the
# "Validate framework" guard and the dsv4-only-dynamo-vllm guard
# further below are srtctl-specific and are never reached from here).
# MODEL_PATH is already resolved above (the dsv4/fp4 branch); llm-d
# additionally needs MODEL_NAME (served-model-name), which this file
# doesn't otherwise set outside the srtctl block - reuse $MODEL (the
# master-config `model:` field), matching this file's own
# `export SERVED_MODEL_NAME=$MODEL` convention below.
if [[ ! ( "$MODEL_PREFIX" == "dsv4" && "$PRECISION" == "fp4" ) ]]; then
echo "Unsupported MODEL_PREFIX/PRECISION for llmd-vllm on B200: $MODEL_PREFIX/$PRECISION" >&2
exit 1
fi
export MODEL_NAME="$MODEL"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The new llmd-vllm block never exports SLURM_PARTITION/SLURM_ACCOUNT before handing off to bash "$BENCH_SCRIPT", which forks submit.sh (a separate process) whose check_env SLURM_ACCOUNT/check_env SLURM_PARTITION require them in the environment; every B200 llmd-vllm job submission fails immediately with "Error: SLURM_ACCOUNT not set". Fix: add export SLURM_PARTITION SLURM_ACCOUNT in the llmd-vllm block, mirroring the sibling tilert branch a few lines below (line 249) which does exactly this before its own subprocess handoff.

Extended reasoning...

SLURM_PARTITION/SLURM_ACCOUNT are set as plain (non-exported) shell vars at the top of the file (lines 5-6) and no workflow sets them as job-level env, so they exist only in this script's local scope. The llmd-vllm branch runs JOB_ID=$(bash "$BENCH_SCRIPT") which execs dsv4_fp4_b200_llmd-vllm-agg.sh/disagg.sh, which runs bash ./submit.sh ... - a brand-new process that only inherits exported vars. submit.sh's check_env SLURM_ACCOUNT / check_env SLURM_PARTITION (both required first) then exit 1 before sbatch ever runs. Contrast with the tilert branch immediately below, which does export SLURM_PARTITION SLURM_ACCOUNT right before its own exec bash "$TILERT_DISAGG" handoff - the same pattern the new block omits.

Verification: normal — the new llmd-vllm block forks a subprocess without exporting the SLURM credentials that subprocess requires. runners/launch_b200-nscale-compat.sh:5-6 set SLURM_PARTITION="${SLURM_PARTITION:-batch_1}" / SLURM_ACCOUNT="${SLURM_ACCOUNT:-benchmark}" as plain (non-exported) vars. The new llmd-vllm branch runs JOB_ID=$(bash "$BENCH_SCRIPT") which execs… | normal — new code…

Comment thread perf-changelog.yaml Outdated
Comment on lines 6391 to 6417
- config-keys:
- dsv4-fp4-b200-llmd-vllm-agentx
description:
- "Add DeepSeek-V4-Pro FP4 B200 llmd-vllm P/D disagg benchmark (1P DEP8 + 1D DEP8, 2 nodes / 16 GPUs), porting the agentX (llm-manifesto/agentx-mvp) EPP scheduling config and per-role vLLM tuning"
- "Wire a FRAMEWORK==llmd-vllm dispatch into runners/launch_b200-dgxc-slurm.sh (self-contained, bypasses the srtctl/srt-slurm path used by the other B200 multinode frameworks)"
- "Pin a B200-only EPP/pd-sidecar v0.10.0 binary override (LLMD_BIN_DIR) for the disagg-profile-handler deciders: EPP shape; GB200's shared v0.9.0 binaries/path are untouched"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER

- config-keys:
- dsv4-fp4-b200-llmd-vllm-agentx-agg
description:
- "Add DeepSeek-V4-Pro FP4 B200 llmd-vllm aggregated benchmark (TP8 and DEP8, 1 node / 8 GPUs each, one engine does both prefill and decode), porting the agentX (llm-manifesto) ix-agg-base/agg-tp8 and ix-agg-dep-base/agg-dep8 per-role vLLM tuning"
- "Extend server.sh/job.slurm with an aggregated (DECODE_NODES=0) mode: no decode role, no pd-sidecar, no --kv_transfer_config, and the sole engine's rank 0 takes over the EPP/Envoy/bench coordinator duties normally owned by the decode leader"
- "Add a minimal single-schedulingProfile EPP config for aggregated recipes (no disagg-profile-handler needed); uses the existing default v0.9.0 EPP/pd-sidecar binaries, no router bump required"
- "Branch runners/launch_b200-dgxc-slurm.sh's llmd-vllm wrapper-script selection on DISAGG (true -> disagg wrapper, false -> new aggregated wrapper)"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER

- config-keys:
- dsv4-fp4-b200-llmd-vllm-agentx
description:
- "Add Mooncake prefix-cache KV store for the disagg DEP8 arm at concurrencies [52, 72]"
- "Extend server.sh to parse a `mooncake.store_config` key from llm-d recipe YAMLs: writes /tmp/mooncake_config.json, sets MOONCAKE_CONFIG_PATH, and wires MultiConnector (NixlConnector + MooncakeStoreConnector) instead of plain NixlConnector; prefill uses kv_both, decode kv_consumer with lookup disabled"
- "Add dsv4-fp4-b200-agentx-1p1d-dep8-dep8-mooncake.yaml recipe with Mooncake store_config (P2PHANDSHAKE embedded, 140 GB segment, mlx5_0..3 RDMA) and MC_* env vars, mirroring the dynamo-vllm agentic GB200 disagg recipe"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2719

- config-keys:
- kimik3-fp4-mi355x-atom-agentic-mtp

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The three new dsv4-fp4-b200-llmd-vllm-agentx* entries are spliced in before pre-existing entries (e.g. kimik3-fp4-mi355x-atom-agentic-mtp) instead of appended at the end of the file, violating AGENTS.md's explicit "The file is append-only ... append only at the tail" rule for this byte-sensitive file. Fix: move the new entries to the actual end of perf-changelog.yaml, after the last existing entry (pr/2787), preserving all other bytes.

Extended reasoning...

AGENTS.md line 22 states perf-changelog.yaml is append-only and byte-sensitive, requiring new entries appended only at the tail. git diff shows the three new config-keys blocks (dsv4-fp4-b200-llmd-vllm-agentx x2, plus the Mooncake entry with pr-link 2719) inserted at line ~6391, immediately before the pre-existing kimik3-fp4-mi355x-atom-agentic-mtp entry that continues to line 6609 in the base file - i.e. mid-file insertion, not appended after the last entry (base tail, pr/2650). This is a direct, mechanical violation of the stated repo convention for this file.

Verification: normal. The three new dsv4-fp4-b200-llmd-vllm-agentx* entries are inserted mid-file, not appended at the tail. In the head file perf-changelog.yaml, they sit at lines 6391-6415 (after the pr/2650 entry ending at line 6389) and directly before the pre-existing kimik3-fp4-mi355x-atom-agentic-mtp entry (pr/2716) at line 6417, which continues to the true tail entry pr/2787 (line ~6660). AGENTS.md…

Comment thread perf-changelog.yaml Outdated
- "Add DeepSeek-V4-Pro FP4 B200 llmd-vllm P/D disagg benchmark (1P DEP8 + 1D DEP8, 2 nodes / 16 GPUs), porting the agentX (llm-manifesto/agentx-mvp) EPP scheduling config and per-role vLLM tuning"
- "Wire a FRAMEWORK==llmd-vllm dispatch into runners/launch_b200-dgxc-slurm.sh (self-contained, bypasses the srtctl/srt-slurm path used by the other B200 multinode frameworks)"
- "Pin a B200-only EPP/pd-sidecar v0.10.0 binary override (LLMD_BIN_DIR) for the disagg-profile-handler deciders: EPP shape; GB200's shared v0.9.0 binaries/path are untouched"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Two new changelog entries (lines 6397, 6406) use pr-link .../pull/PLACEHOLDER, which is not a valid placeholder: validate_perf_changelog.py's PR_LINK_PLACEHOLDERS only allows the literal "XXX" or ".../pull/XXX", and prepare_perf_changelog_merge.py's canonicalize_appended_links() raises ChangelogValidationError("appended entry N has unexpected pr-link") for anything else, including "PLACEHOLDER". Fix: replace both occurrences with the "XXX" placeholder convention (or the real PR number) so the merge-prep automation can canonicalize them.

Extended reasoning...

utils/prepare_perf_changelog_merge.py:canonicalize_appended_links iterates the appended entries (index >= len(base_entries)) and for each one whose pr-link != expected_link, requires link in PR_LINK_PLACEHOLDERS ({"XXX", "https://github.com/SemiAnalysisAI/InferenceX/pull/XXX"}); the two entries added by this diff (perf-changelog.yaml:6397 and :6406) instead literally read https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER. That string is neither the eventual canonical pull/<pr_number> link nor a recognized placeholder, so the if link not in PR_LINK_PLACEHOLDERS: raise ChangelogValidationError(...) branch fires when the merge-prep bot/step runs on this PR, blocking automatic pr-link canonicalization that the repo's changelog workflow depends on.

Verification: normal. The two entries added by this diff at perf-changelog.yaml:6397 and :6406 both use pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER. That string is not a valid placeholder: utils/validate_perf_changelog.py:24-27 defines PR_LINK_PLACEHOLDERS = {"XXX", "https://github.com/SemiAnalysisAI/InferenceX/pull/XXX"}, and CANONICAL_PR_LINK (:21-23) only matches… | normal.…

@@ -0,0 +1,133 @@
# DeepSeek-V4-Pro (FP4) on B200, aggregated TP8 (one engine does both

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit (optional): all 4 new recipe files are named with a gb200 prefix but are actually B200 recipes (each file's own header says "on B200", and they're wired from the B200-only dsv4-fp4-b200-llmd-vllm-agentx[-agg] master-config keys via runner: b200-multinode), unlike the pre-existing dsv4-fp4-gb200-*.yaml files which really are GB200. Fix: rename to an b200-... prefix so filename matches platform and doesn't collide/confuse with the real GB200 recipes. sweep:grep -l "on B200" benchmarks/multi_node/llm-d-recipes/agentic/*gb200-*.yaml

Extended reasoning...

A future engineer or agent grepping llm-d-recipes for gb200-* to find actual GB200 configs will also match these 4 B200-only files (agg-gb200-dep8-dspark-agentic.yaml, agg-gb200-dep8-dspark-mooncake-agentic.yaml, agg-gb200-tp8-dspark-agentic.yaml, disagg-gb200-1p1d-dep8-dep8-dspark-agentic.yaml), risking them being edited/copied as if they were GB200 topology (4 GPUs/node, 2-node DEP8 spans) when they are actually single-node 8-GPU B200 configs — each file's own header comment explicitly contradicts its filename. No functional breakage today since CONFIG_FILE references are consistent, but it's a latent source of misdirected edits.

Verification: nit. The naming inconsistency is factually real. The 4 new files in benchmarks/multi_node/llm-d-recipes/agentic/ carry a gb200 prefix (agg-gb200-tp8-dspark-agentic.yaml, agg-gb200-dep8-dspark-agentic.yaml, agg-gb200-dep8-dspark-mooncake-agentic.yaml, disagg-gb200-1p1d-dep8-dep8-dspark-agentic.yaml), yet every file's own header opens "DeepSeek-V4-Pro (FP4) on B200" and states "B200 = 8… | nit…

Comment on lines +125 to +156
llmd_enroot_uri_for_image() {
local image="$1"
local image_without_digest="$image"
local digest=""
local first_component registry repository

if [[ "$image" == *@sha256:* ]]; then
image_without_digest="${image%@*}"
digest="${image##*@}"
fi

first_component="${image_without_digest%%/*}"
if [[ "$image_without_digest" == */* && ( "$first_component" == *.* || "$first_component" == *:* || "$first_component" == "localhost" ) ]]; then
registry="$first_component"
repository="${image_without_digest#*/}"
else
registry="registry-1.docker.io"
repository="$image_without_digest"
fi

if [[ -z "$digest" ]]; then
if [[ "$registry" == "registry-1.docker.io" ]]; then
printf 'docker://%s\n' "$image"
else
printf 'docker://%s#%s\n' "$registry" "$repository"
fi
return
fi
if [[ "$registry" == "registry-1.docker.io" && "$repository" != */* ]]; then
repository="library/$repository"
fi
printf 'docker://%s#%s:%s\n' "$registry" "$repository" "$digest"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) llmd_enroot_uri_for_image() doesn't strip the tag before appending the digest for a repo:tag@ sha256:digest image, producing a malformed enroot URI docker://registry#repo:tag:sha256:hash (three colons) instead of repo:digest, so enroot import fails for exactly the tag@ digest images this function's own comment says it exists to support. Fix: when a digest is present, derive repository from the image path with any :tag stripped before appending :$digest.

Extended reasoning...

image_without_digest keeps the original :tag (only the @ digest suffix is removed via ${image%@*}), so repository is set to e.g. rh-ee-imarkov/llm-d-nokube-vllm:vllm0.26. When digest is non-empty, the final printf 'docker://%s#%s:%s\n' "$registry" "$repository" "$digest" appends :sha256:<hash> onto that, yielding docker://quay.io#rh-ee-imarkov/llm-d-nokube-vllm:vllm0.26:sha256:<hash>. This is the exact tag@ digest format already used for the sibling dsv4-fp4-gb200-llmd-vllm image in configs/nvidia-master.yaml (line ~4426); if a b200-multinode llmd-vllm config is later pinned the same way (as the file's own TODO about switching to an 'official llmd image' suggests), enroot import will reject the malformed reference and the job submission (llmd_import_squash, line 165) exits 1 before any node is launched.

Verification: Severity: nit (latent — not triggered by any shipped image). The code defect is real: the new llmd_enroot_uri_for_image() in runners/launch_b200-nscale-compat.sh is a copy of enroot_uri_for_image() in runners/launch_gb200-nv.sh but drops the tag-stripping lines the reference has (launch_gb200-nv.sh:51-58: repository_dir/repository_name and repository_name="${repository_name%%:*}"). Because…

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit af3818d. Configure here.

Comment thread runners/launch_b200-nscale-compat.sh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant